Encode log recorder name when redirecting#26331
Conversation
MarkEWaite
left a comment
There was a problem hiding this comment.
Thanks for the pull request.
Format violations show that you either did not read the contributing guide or you missed the section that describes automated formatting. Read the guide, follow the instructions, and submit changes to correct the formatting issue.
I'm marking the pull request as "Draft". When you've fixed the issues and the checks (ci.jenkins.io and others) are passing, you can mark it as ready for review.
MarkEWaite
left a comment
There was a problem hiding this comment.
Successfully tested with a few different UTF-8 strings, including:
- A person 人 symbol from CJK - 3 byte UTF
- Aña - 2 byte Latin-1 supplement
- Happy - 😊- 4 byte emoji
- Hello, world € - 3 byte common symbol
- I ♥ logs - 4 byte common symbol
- Léon ama logs anche - 2 byte Latin-1 supplement
Confirmed with the debugger that the test code exercises the modified line
This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.
/label ready-for-merge
|
@Flamki you removed the maintainer checklist from the pull request template. In the future, that mistake will result in the pull request being immediately closed. Please restore the maintainer checklist from the pull request template. |
|
Thanks for the review and guidance. I restored the maintainer checklist in the PR description and cleaned up the formatting/template sections to match the contribution requirements. The fix and regression test are unchanged. I’ll wait for CI to finish and for maintainer merge once the waiting window completes. |
|
Congratulations on getting your very first Jenkins core pull request merged 🎉🥳 |
Fixes #26318
Testing done
createLogRecorderWithNonAsciiNameinLogRecorderManagerTest.Journal d’accèsand verifies that the configure URL resolves with an encoded path segment.Screenshots (UI changes only)
Before
N/A (no UI layout change)
After
N/A (no UI layout change)
Proposed changelog entries
Proposed changelog category
/label bug
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate. (No new public API in this PR.)@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable. (No new deprecations in this PR.)evalto ease future introduction of Content Security Policy (CSP) directives (see documentation). (No JS/UI behavior change in this PR.)Desired reviewers
@jenkinsci/core-pr-reviewers
Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered.